Improve object_store docs #4978

tustvold · 2023-10-23T17:32:29Z

Which issue does this PR close?

Closes #3888
Relates to #4922 (review)
Part of #4879

Rationale for this change

This works on improving the docs for object_store, with a particular view to:

Better document the functionality
Articulate the vision of the crate and how it is differentiated from other offerings

What changes are included in this PR?

Are there any user-facing changes?

Xuanwo

Great document! Inspired me a lot.

alamb

I love it -- thank you @tustvold -- very nice. 👏

I have a bunch of small editorial improvement suggestions but nothing that I think would prevent this PR from merging as is

alamb · 2023-10-27T19:59:18Z

object_store/src/lib.rs

 //!
-//! 3. Stable and predictable governance via the [Apache Arrow] project.
+//! 3. Support for advanced functionality, including [ACID] reads


I suggest you don't use ACID here as it is a database term and not as commonly applied to object stores in my experience.

For example, I think the claim that the D in ACID for Durable counts as "advanced" functionality for object stores / their APIs is confusing. It is also unclear to me what this crates adds for Isolation in the classic database sense of the word.

Perhaps a better term would be "atomic reads/writes" which also highlights the feature I think you are hinting at

I was trying to allude that you could build ACID transactions on top, but fair enough will change

alamb · 2023-10-27T20:00:21Z

object_store/src/lib.rs

+//!
+//! 4. Stable and predictable governance via the [Apache Arrow] project
+//!
+//! 5. Very low dependency footprint


Suggested change

//! 5. Very low dependency footprint

//! 5. Small dependency footprint (depends on only a small number of common crates)

alamb · 2023-10-27T20:01:35Z

object_store/src/lib.rs

+//! # Why not a Filesystem Interface?
+//!
+//! Whilst this crate does provide a [`BufReader`], the [`ObjectStore`] interface mirrors the APIs
+//! of object stores and not filesystems.


I think it might help to give an example of what a filesystem API means. Perhaps something like

Suggested change

//! of object stores and not filesystems.

//! of object stores and not filesystems. For example, it purposely does not offer a `Seek` like API.

object_store/src/lib.rs

alamb · 2023-10-27T20:07:20Z

object_store/src/lib.rs

+//! A common pattern, especially when reading structured datasets, is to need to fetch
+//! multiple ranges of a particular object.


Suggested change

//! A common pattern, especially when reading structured datasets, is to need to fetch

//! multiple ranges of a particular object.

//! A common pattern, especially when reading structured datasets, is to fetch

//! multiple, potentially discontiguous, ranges of a particular object.

alamb · 2023-10-27T20:10:58Z

object_store/src/lib.rs

+//!         Ok(match self.entries.get_mut(path) {
+//!             Some(e) => match e.refreshed_at.elapsed() < Duration::from_secs(10) {
+//!                 true => e.data.clone(), // Return cached data
+//!                 false => {


Suggested change

//! false => {

//! false => { // check if remote version has changed

alamb · 2023-10-27T20:11:18Z

object_store/src/lib.rs

+//!                     e.data.clone()
+//!                 }
+//!             },
+//!             None => {


Suggested change

//! None => {

//! None => { // not cached, fetch value

alamb · 2023-10-27T20:11:43Z

object_store/src/lib.rs

+//!
+//! # Conditional Put
+//!
+//! The default behaviour when writing data is to upsert any existing object at the given path.


Suggested change

//! The default behaviour when writing data is to upsert any existing object at the given path.

//! The default behaviour when writing data is to upsert any existing object at the given path, overwriting any previous values.

alamb · 2023-10-27T20:12:10Z

object_store/src/lib.rs

+//! #   Arc::new(InMemory::new())
+//! # }
+//! # fn do_update(b: Bytes) -> Bytes {b}
+//! # async fn conditional_put() {


It might be helpful to have this name exposed

Suggested change

//! # async fn conditional_put() {

//! async fn conditional_put() {

I opted to add a comment

alamb · 2023-10-27T20:13:05Z

object_store/src/lib.rs

+//!     // Attempt to commit transaction
+//!     match store.put_opts(&path, new, PutMode::Update(version).into()).await {
+//!         Ok(_) => break, // Successfully committed
+//!         Err(Error::Precondition { .. }) => continue, // Transaction conflict, try again


Suggested change

//! Err(Error::Precondition { .. }) => continue, // Transaction conflict, try again

//! Err(Error::Precondition { .. }) => continue, // object has been changed, try again

github-actions bot added the object-store Object Store Interface label Oct 23, 2023

tustvold force-pushed the object-store-docs branch 2 times, most recently from 0b14d77 to e274581 Compare October 27, 2023 11:41

tustvold added the development-process Related to development process of arrow-rs label Oct 27, 2023

Improve object_store docs

8826bb7

tustvold force-pushed the object-store-docs branch from e274581 to 8826bb7 Compare October 27, 2023 11:50

tustvold marked this pull request as ready for review October 27, 2023 11:50

tustvold mentioned this pull request Oct 27, 2023

Consider looking into Apache OpenDAL as an alternative to object_store #3888

Closed

Xuanwo approved these changes Oct 27, 2023

View reviewed changes

Document configuration system

16e0156

alamb approved these changes Oct 27, 2023

View reviewed changes

Review feedback

0bcc2a2

tustvold force-pushed the object-store-docs branch from 17275b6 to 0bcc2a2 Compare October 30, 2023 11:29

tustvold merged commit cc23cac into apache:master Oct 30, 2023
13 checks passed

tustvold mentioned this pull request Nov 2, 2023

Object Store Doctest Failure with Default Features #5025

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve object_store docs #4978

Improve object_store docs #4978

tustvold commented Oct 23, 2023 •

edited

Loading

Xuanwo left a comment •

edited

Loading

alamb left a comment

alamb Oct 27, 2023

tustvold Oct 30, 2023

alamb Oct 27, 2023

alamb Oct 27, 2023

alamb Oct 27, 2023

alamb Oct 27, 2023

alamb Oct 27, 2023

alamb Oct 27, 2023

alamb Oct 27, 2023

tustvold Oct 30, 2023

alamb Oct 27, 2023

	//! 5. Very low dependency footprint
	//! 5. Small dependency footprint (depends on only a small number of common crates)

	//! of object stores and not filesystems.
	//! of object stores and not filesystems. For example, it purposely does not offer a `Seek` like API.

		//! A common pattern, especially when reading structured datasets, is to need to fetch
		//! multiple ranges of a particular object.

	//! false => {
	//! false => { // check if remote version has changed

	//! The default behaviour when writing data is to upsert any existing object at the given path.
	//! The default behaviour when writing data is to upsert any existing object at the given path, overwriting any previous values.

	//! # async fn conditional_put() {
	//! async fn conditional_put() {

	//! Err(Error::Precondition { .. }) => continue, // Transaction conflict, try again
	//! Err(Error::Precondition { .. }) => continue, // object has been changed, try again

Improve object_store docs #4978

Improve object_store docs #4978

Conversation

tustvold commented Oct 23, 2023 • edited Loading

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Xuanwo left a comment • edited Loading

Choose a reason for hiding this comment

alamb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tustvold commented Oct 23, 2023 •

edited

Loading

Xuanwo left a comment •

edited

Loading